Demo instrument separation for Sync template #58

ravescovi · 2025-03-10T22:36:10Z

Description

Separates demo_instrument from the rest of bits core

Fixes #56

Type of change

Choose which options apply, and delete the ones which do not apply.

New feature (non-breaking change that adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update
Code maintenance/cleanup

…S/BITS into switch_case_organization

coveralls · 2025-03-10T23:37:06Z

Pull Request Test Coverage Report for Build 13796866742

Details

61 of 62 (98.39%) changed or added relevant lines in 15 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage increased (+0.1%) to 85.492%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/bits/demo_instrument/startup.py	16	17	94.12%

Totals
Change from base Build 13774228748:	0.1%
Covered Lines:	713
Relevant Lines:	834

💛 - Coveralls

src/bits/callbacks/nexus_data_file_writer.py

prjemian · 2025-03-11T02:35:37Z

src/bits/callbacks/spec_data_file_writer.py

@@ -17,8 +17,8 @@
 import apstools.callbacks
 import apstools.utils

-from ..core.run_engine_init import RE
-from ..utils.config_loaders import iconfig
+from bits.core.run_engine_init import RE


Looking for similar enable check, as in nexus above.

I don't think an enable belongs here. The run engine should always be enabled.

prjemian · 2025-03-11T02:38:04Z

src/bits/core/catalog_init.py

@@ -10,7 +10,7 @@

 import databroker

-from ..utils.config_loaders import iconfig
+from bits.utils.config_loaders import iconfig


Getting puzzled here. If this part is in bits (and not instrument), then keep the relative path.

Comment applies below, too

src/bits/demo_instrument/configs/iconfig.yml

prjemian · 2025-03-11T02:41:55Z

src/bits/demo_instrument/configs/iconfig.yml

@@ -64,7 +67,7 @@ OPHYD:
    ### Control layer for ophyd to communicate with EPICS.
    ### Default: PyEpics


No need to describe the default if value is always provided.

Then we remove from the iconfig

prjemian · 2025-03-11T02:55:36Z

Taking a bit of time to understand until reading this:

BITS/src/bits/utils/create_new_instrument.py

Lines 3 to 9 in 54230fb

    
           Script to create a new instrument based on the 'src/bits/demo_instrument' template 
        
           folder. 
        
           This script copies the template instrument folder into a new directory with the 
        
           provided instrument name (which must be a valid Python package name) and updates 
        
           the pyproject.toml file with an entry under [tool.instruments] reflecting the 
        
           new instrument's relative path.

Probably need to try out the full process to evaluate. How to use this branch as a template for a new repo?

Co-authored-by: Pete R Jemian <[email protected]>

ravescovi · 2025-03-11T03:56:10Z

I think a lot of the comments are derived from #47 (which was used as base for this commit here). I will wait for that one to be merged before fixing the comments above.

ravescovi · 2025-03-11T04:00:08Z

Taking a bit of time to understand until reading this:

BITS/src/bits/utils/create_new_instrument.py

Lines 3 to 9 in 54230fb

Script to create a new instrument based on the 'src/bits/demo_instrument' template

folder.

This script copies the template instrument folder into a new directory with the

provided instrument name (which must be a valid Python package name) and updates

the pyproject.toml file with an entry under [tool.instruments] reflecting the

new instrument's relative path.

Probably need to try out the full process to evaluate. How to use this branch as a template for a new repo?

This is not working yet. It is just a place holder for now.

…emo_instrument

…ting if a variable is imported of not based on iconfig

…emo_instrument

ravescovi · 2025-03-11T19:55:24Z

I suggest we stop this PR here and open a new one for the init / sync functionality.

prjemian · 2025-03-11T21:19:41Z

I believe the only we can test this as a template is to merge this PR. The page to create a new repo from this template doesn't show an option for a different branch. Either default or all:

Any additional work will go into a new issue.

ravescovi · 2025-03-11T21:29:43Z

I deactivated the python bits of the init behaviour but we could replicate it locally. Feel free to merge if this PR is on the direction you want to go.

…

On Tue, Mar 11, 2025 at 4:20 PM Pete R Jemian ***@***.***> wrote: I believe the only we can test this as a template is to merge this PR. The page to *create a new repo from this template* doesn't show an option for a different branch. Either default or all: image.png (view on web) <https://github.com/user-attachments/assets/2b1a00ba-57a8-461a-a797-457fcf89dae6> Any additional work will go into a new issue. — Reply to this email directly, view it on GitHub <#58 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEFMPQ6N7BG7IGNUULIUHML2T5HQHAVCNFSM6AAAAABYXMQJZ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMJVG4ZTEMBUHE> . You are receiving this because you were assigned.Message ID: ***@***.***> [image: prjemian]*prjemian* left a comment (BCDA-APS/BITS#58) <#58 (comment)> I believe the only we can test this as a template is to merge this PR. The page to *create a new repo from this template* doesn't show an option for a different branch. Either default or all: image.png (view on web) <https://github.com/user-attachments/assets/2b1a00ba-57a8-461a-a797-457fcf89dae6> Any additional work will go into a new issue. — Reply to this email directly, view it on GitHub <#58 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEFMPQ6N7BG7IGNUULIUHML2T5HQHAVCNFSM6AAAAABYXMQJZ6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMJVG4ZTEMBUHE> . You are receiving this because you were assigned.Message ID: ***@***.***>

MDecarabas and others added 22 commits March 6, 2025 13:05

set up is in 2 steps so RE no longer crashes stuff

01d880a

fixed issue with error logging magics

1a0c9e9

make devices works no error

fd9eb39

organizing factory device

107f5c1

Merge branch 'main' into switch_case_organization

28d7d71

fix tests

eff48dd

Merge branch 'switch_case_organization' of https://github.com/BCDA-AP…

cf85fe6

…S/BITS into switch_case_organization

Update README.md

4c54b5c

Merge branch 'main' into switch_case_organization

edd1f7c

added features to iconfig

9fd1744

rebase

dfbf7f2

make devices works no error

ab38d00

rebase

e125b66

MNT #37 following repo rename

232c542

always enabled

8e584ee

Merge branch 'main' into switch_case_organization

4da36b9

new instrument

2b8789c

readded file

954fb1f

demo_instrument

b25ba14

fix relative imports

edf614b

precommit

07711e8

fix tests

9e80a8d

ravescovi changed the title ~~Demo instrument~~ Demo instrument separation for Sync template Mar 10, 2025

ravescovi added 2 commits March 10, 2025 17:58

fix actions

5362261

add README

aaef8c2

ravescovi force-pushed the demo_instrument branch from 453bdd2 to aaef8c2 Compare March 10, 2025 23:21

fix??

cfac9ef

add create_instrument to init_repo

26fea41

ravescovi requested a review from prjemian March 10, 2025 23:45

prjemian reviewed Mar 11, 2025

View reviewed changes

src/bits/callbacks/nexus_data_file_writer.py Outdated Show resolved Hide resolved

prjemian reviewed Mar 11, 2025

View reviewed changes

src/bits/demo_instrument/configs/iconfig.yml Show resolved Hide resolved

prjemian reviewed Mar 11, 2025

View reviewed changes

src/bits/demo_instrument/configs/iconfig.yml Outdated Show resolved Hide resolved

prjemian reviewed Mar 11, 2025

View reviewed changes

ravescovi and others added 2 commits March 10, 2025 22:53

Update .github/workflows/init_repo.yml

e9b84d7

Co-authored-by: Pete R Jemian <[email protected]>

Update docs/source/demo.ipynb

ac3e7f0

Co-authored-by: Pete R Jemian <[email protected]>

MDecarabas and others added 15 commits March 11, 2025 12:39

finished iconfig cleanup ...for nbow

e639a94

pre-commited

dfb0118

will not fix but interesting to observe

f800268

Merge remote-tracking branch 'origin/switch_case_organization' into d…

9b47a7a

…emo_instrument

hot fix since startup is being imported at top of test and we are tes…

36177a3

…ting if a variable is imported of not based on iconfig

sync

29e9d31

Merge remote-tracking branch 'origin/switch_case_organization' into d…

8b38e51

…emo_instrument

WIP test

b6546d0

WIP test

b5900e7

test wip

f414b1e

IS_ON = Enable

b3f7df3

cleaned up readme due to suggestions

3b00fcc

Merge remote-tracking branch 'origin/switch_case_organization' into d…

6e57afd

…emo_instrument

hide init_repo placeholders

80c7200

add metadata

8f392ed

ravescovi marked this pull request as ready for review March 11, 2025 21:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Demo instrument separation for Sync template #58

Demo instrument separation for Sync template #58

ravescovi commented Mar 10, 2025

coveralls commented Mar 10, 2025 •

edited

Loading

prjemian Mar 11, 2025

MDecarabas Mar 11, 2025

prjemian Mar 11, 2025

prjemian Mar 11, 2025

prjemian Mar 11, 2025 •

edited

Loading

MDecarabas Mar 11, 2025

prjemian commented Mar 11, 2025

ravescovi commented Mar 11, 2025

ravescovi commented Mar 11, 2025

ravescovi commented Mar 11, 2025

prjemian commented Mar 11, 2025

ravescovi commented Mar 11, 2025 via email

		@@ -64,7 +67,7 @@ OPHYD:
		### Control layer for ophyd to communicate with EPICS.
		### Default: PyEpics

Demo instrument separation for Sync template #58

Are you sure you want to change the base?

Demo instrument separation for Sync template #58

Conversation

ravescovi commented Mar 10, 2025

Description

Type of change

coveralls commented Mar 10, 2025 • edited Loading

Pull Request Test Coverage Report for Build 13796866742

Details

💛 - Coveralls

prjemian Mar 11, 2025

Choose a reason for hiding this comment

MDecarabas Mar 11, 2025

Choose a reason for hiding this comment

prjemian Mar 11, 2025

Choose a reason for hiding this comment

prjemian Mar 11, 2025

Choose a reason for hiding this comment

prjemian Mar 11, 2025 • edited Loading

Choose a reason for hiding this comment

MDecarabas Mar 11, 2025

Choose a reason for hiding this comment

prjemian commented Mar 11, 2025

ravescovi commented Mar 11, 2025

ravescovi commented Mar 11, 2025

ravescovi commented Mar 11, 2025

prjemian commented Mar 11, 2025

ravescovi commented Mar 11, 2025 via email

coveralls commented Mar 10, 2025 •

edited

Loading

prjemian Mar 11, 2025 •

edited

Loading